-
Notifications
You must be signed in to change notification settings - Fork 3
feat(connectors): add OAuth connector resource #189
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
🚀 Package Preview Available!Install this PR's preview build with npm: npm i @base44-preview/cli@0.0.31-pr.189.ff1c40dPrefer not to change any import paths? Install using npm alias so your code still imports npm i "base44@npm:@base44-preview/cli@0.0.31-pr.189.ff1c40d"Or add it to your {
"dependencies": {
"base44": "npm:@base44-preview/cli@0.0.31-pr.189.ff1c40d"
}
}
Preview published to npm registry — try new features instantly! |
c3543de to
c1bb2d1
Compare
7bfb43f to
7b418ac
Compare
Code reviewNo issues found. Checked for bugs and CLAUDE.md compliance. |
| "hubspot", | ||
| "linkedin", | ||
| "tiktok", | ||
| ] as const; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Might be annoying that we have to create a new connectorSchema, update the ConnectorResourceSchema and add the name to KnownIntegrationTypes, Maybe we can enforce this somehow with typescript.. right now the KnownIntegrationTypes is not enforced, meaning there can be typos / missing stuff from this list which will cause issues
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah I agree, talked with Neta about this, we should start syncing the server schema into the CLI but this will happen at a later stage, for now this is mainly a hint for the agent
| error: z.string().nullable().optional(), | ||
| error_message: z.string().nullable().optional(), | ||
| other_user_email: z.string().nullable().optional(), | ||
| }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use .tranform to transform the response the camelCase (there's examples for this in other resources)
|
|
||
| export type RemoveConnectorResponse = z.infer< | ||
| typeof RemoveConnectorResponseSchema | ||
| >; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
related to the entire file: I know we have this issue in many places - can we not export stuff that are not used anywhere? Go over this file and just check what is actually used and export only that
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
agreed, how about we add Knip?
|
|
||
| let finalStatus: ConnectorOAuthStatus = "PENDING"; | ||
|
|
||
| await pWaitFor( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
polling should not happen in the core/ folder, Core/ should only expose api / schema/ config stuff.
Polling and using pWaitFor should happen on the CLI side - you can see an example in login-flow.ts
| return result.data; | ||
| } | ||
|
|
||
| export async function setConnector( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: maybe we can give this a better name? enableConnector? updateConnector? i feel set is not clear to me
| connector.type, | ||
| connector.scopes ?? [] | ||
| ); | ||
| results.push(setResponseToResult(connector.type, response)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe it's because i don't like "setConnector" name, but this name is also not clear to me - maybe something like "getConnectorSyncResult"?
| const upstream = await listConnectors(); | ||
| const localTypes = new Set(connectors.map((c) => c.type)); | ||
|
|
||
| for (const connector of connectors) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think basic flow comments / splitting into function might help in this function.
// 1. Update local connector to remote
// 2. Delete remote connector that are not in local project
| const types = new Set<string>(); | ||
| for (const connector of connectors) { | ||
| if (types.has(connector.type)) { | ||
| throw new InvalidInputError( | ||
| `Duplicate connector type "${connector.type}"`, | ||
| { | ||
| hints: [ | ||
| { | ||
| message: `Remove duplicate connectors with type "${connector.type}" - only one connector per type is allowed`, | ||
| }, | ||
| ], | ||
| } | ||
| ); | ||
| } | ||
| types.add(connector.type); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
extract to another function called something like assertNoDuplicateConnectors
| export function getConnectorsPushCommand(context: CLIContext): Command { | ||
| return new Command("push") | ||
| .description( | ||
| "Push local connectors to Base44 (syncs scopes and removes unlisted)" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure this description is really clear, "removed unlisted"?
| const needsOAuth = results.filter(isPendingOAuth); | ||
| let outroMessage = "Connectors pushed to Base44"; | ||
|
|
||
| if (needsOAuth.length > 0) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can we flip and exit early?
if (needsOauth.length === 0) {
return { outroMessage };
}
| let outroMessage = "Connectors pushed to Base44"; | ||
|
|
||
| if (needsOAuth.length > 0) { | ||
| log.info(""); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we really need an extra space?
| log.info(`\nOpening browser for ${connector.type}...`); | ||
|
|
||
| const oauthResult = await runTask( | ||
| `Waiting for ${connector.type} authorization...`, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just to add some ticks on the name of the connector
| `Waiting for ${connector.type} authorization...`, | |
| `Waiting for '${connector.type}' authorization...`, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same for the earlier log
| const oauthResult = await runTask( | ||
| `Waiting for ${connector.type} authorization...`, | ||
| async () => { | ||
| return await runOAuthFlow({ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wrote it inside this function but i will mention it here too - the pwaitFor and polling logic should be implemented here, and just call the correct API from the core folder.
| successMessage: `${connector.type} authorization complete`, | ||
| errorMessage: `${connector.type} authorization failed`, | ||
| } | ||
| ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
consider adding try/catch - if one fails, do we want to entire process to fail or just keep going? i think keep going makes sense..
kfirstri
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added some comments, main issue i got is the pWaitFor call - it should be implemented inside CLI and not in core (like login-flow.ts)
Add connector resource module supporting 12 OAuth providers: googlecalendar, googledrive, gmail, googlesheets, googledocs, googleslides, slack, notion, salesforce, hubspot, linkedin, tiktok. - Zod discriminated union schema with type discriminator per provider - JSDoc links to official OAuth scope documentation for each provider - JSONC file reading with validation (filename must match type field) - API response schemas for upstream connector state - Unit tests with fixtures for valid, invalid, and mismatched connectors Part of: #184 Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Add API client methods for OAuth connector operations: - listConnectors: list all connectors for current app - syncConnector: sync connector with exact scope matching - getOAuthStatus: poll OAuth authorization status - removeConnector: remove a connector integration Also update response schemas and clean up verbose comments. Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com>
* feat(connectors): implement push logic for syncing connectors Add pushConnectors function that: - Syncs all local connectors via /sync endpoint - Removes upstream-only connectors not in local config - Returns typed results (synced, removed, needs_oauth, error) Includes unit tests covering all scenarios. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * feat(connectors): add OAuth flow handling with browser redirect and polling (#192) * feat(connectors): add OAuth flow handling with browser redirect and polling Add runOAuthFlow function that: - Opens OAuth redirect URL in browser - Polls getOAuthStatus until ACTIVE or FAILED - Returns PENDING on timeout (5 minutes) Uses p-wait-for TimeoutError for robust timeout detection. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * connectors: base44 connectors push (#194) * final connector work sofi 1 * scopes --------- Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Change provider field from closed enum to flexible union that accepts both known providers (googlecalendar, notion, slack, etc.) and any arbitrary provider string. This enables users to configure custom OAuth providers without waiting for first-class Base44 support. Schema changes: - Add GenericConnectorSchema for arbitrary provider types - Update ConnectorResourceSchema to union of specific + generic schemas - Update IntegrationTypeSchema to accept known enum OR any non-empty string - Only reject empty strings Test coverage: - Verify known providers continue to work - Verify arbitrary providers are accepted - Verify empty strings are rejected - All 137 tests passing Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
0d6430f to
3fc2e0c
Compare
|
README check ran. 7 issue(s) found and applied: (1) Added missing |
|
Note: The README fix commit (1bffec2) was created but could not be pushed due to detached HEAD state in the CI environment. To apply the fix, either: (1) Fetch and cherry-pick commit 1bffec2, or (2) Apply the changes shown in the diff above manually. The required changes are: add |
Note
Description
This PR adds OAuth connector support to the Base44 CLI, allowing users to define connector configurations in
connectors/*.jsoncfiles and push them to their Base44 apps. Users can configure 12 supported OAuth providers (Google Calendar, Slack, Notion, etc.) with custom scopes, and the CLI handles the complete OAuth flow with browser-based authorization.Related Issue
Fixes #184
Type of Change
Changes Made
connectorresource type following the existing Resource patternbase44 connectors pushcommand to sync local connector definitions with Base44connectors_push.spec.ts, 506 lines inconnectors.spec.ts)AGENTS.mddocumentation with new connector resource structureTesting
npm test)Checklist
Additional Notes
The implementation follows the established Resource pattern used by entities, functions, and agents. Connectors are read from
connectors/*.jsoncfiles, validated against provider-specific Zod schemas, and synced with the backend via thepushConnectors()function. The OAuth flow handles the complete authorization cycle including browser launch, polling for completion, and status reporting with a comprehensive summary of results.🤖 Generated by Claude | 2026-02-11 16:25 UTC